-
Notifications
You must be signed in to change notification settings - Fork 169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preview api lb profile #3020
Preview api lb profile #3020
Conversation
29af082
to
63d82b1
Compare
63d82b1
to
3f87b5f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked at the logic behind the things, but we should confirm before we release the API definitions BU is okay with our descriptions.
"x-ms-identifiers": [] | ||
}, | ||
"allocatedOutboundPorts": { | ||
"description": "The desired number of allocated SNAT ports per VM. Allowed values are in the range of 0 to 64000 (inclusive). The default value is 1024.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Load balancer gives SNAT ports in multiples of 8. If you provide a value not divisible by 8, the configuration operation is rejected. Each load balancing rule and inbound NAT rule consume a range of eight ports. If a load balancing or inbound NAT rule shares the same range of 8 as another, no extra ports are consumed.
Is 0 really a valid value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When setting allocatedOutboundPorts to 0 Azure will "dynamically" allocate ports, which is "Use the default number of ports" https://learn.microsoft.com/en-us/azure/load-balancer/load-balancer-outbound-connections#preallocatedports.
Please rebase pull request. |
a1b608d
to
d2d81dc
Compare
Which issue this PR addresses:
Fixes https://issues.redhat.com/browse/ARO-3111
What this PR does / why we need it:
This PR adds the LoadBalancerProfile to the preview, admin, and internal API, static validation of the LoadBalancerProfile, and sets default loadBalancerProfile. I had intended to add Dynamic validation but that is probably going to add too much to this PR and makes more sense to add it during implementation of aro update.
The PreviewAPI includes all fields for this feature though only ManagedOutboundIPs is usable during phase 1 of this feature. This was done this way to not require multiple API releases.
Test plan for issue:
Added/updated existing unit tests. I also manually verified some API calls and validated the expected result in cosmos. This will be further tested once the aro update part is implemented.
Is there any documentation that needs to be updated for this PR?
Not yet